fix(sdk): select gRPC native library by platform and arch - #2029
fix(sdk): select gRPC native library by platform and arch#2029Utkal059 wants to merge 3 commits into
Conversation
The JavaScript SDK's UniFFI loader resolves natives from generated/<platform>-<arch>/, but the gRPC loader still selected on process.platform alone and read a flat generated/ path, so it had no way to pick between architectures. Route both loaders through one resolver and stage the gRPC native under the same per-arch directory. When no matching binary is bundled, the resolver now reports the target it looked for and the targets the package actually ships, instead of letting dlopen fail with a "wrong ELF class" message that does not say which architecture was expected.
There was a problem hiding this comment.
Pull request overview
This PR aligns the JavaScript SDK’s native library loading behavior with the multi-arch packaging layout by resolving native binaries from generated/<platform>-<arch>/, and centralizes the platform/arch resolution logic to prevent drift between UniFFI and gRPC loaders.
Changes:
- Introduces a shared
resolveNativeLib()resolver for platform+arch-aware native library lookup and better failure diagnostics. - Updates the gRPC generated client (and its template) to use the shared resolver and the per-arch directory layout.
- Updates the JavaScript SDK Makefile to stage the gRPC native library under
generated/$(NODE_TARGET)/to match runtime lookup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| sdk/javascript/src/payments/uniffi_client.ts | Switches UniFFI native loading to the shared resolver and per-arch layout. |
| sdk/javascript/src/payments/native_lib.ts | Adds a shared platform+arch native resolver with improved error messaging. |
| sdk/javascript/src/payments/_generated_grpc_client.ts | Updates gRPC native loading to use the shared resolver and per-arch layout. |
| sdk/javascript/Makefile | Stages gRPC native libs into generated/<platform>-<arch>/ to match resolver behavior. |
| scripts/generators/code/templates/javascript/grpc_client.ts.j2 | Updates the generator template so regenerated gRPC clients use the shared resolver. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| export function resolveNativeLib(generatedDir: string, libName: string): string { | ||
| const target = `${process.platform}-${process.arch}`; | ||
| const ext = process.platform === "darwin" ? "dylib" : "so"; | ||
| const libPath = path.join(generatedDir, target, `${libName}.${ext}`); |
The SDK requirements list Windows (x64), but the extension mapping only distinguished darwin from everything else, so a win32 runtime reported a .so path it would never have loaded. Map win32 to .dll so the "not bundled" diagnostic names the file the platform actually uses. No behaviour change on Linux or macOS, and no Windows native is built today — this only affects which filename the error reports.
|
CI note on the failing JavaScript is the only SDK this PR touches, and that run exercises the changed path end to end — The diff touches no Rust, Kotlin, or workflow files, so I've left CI alone rather than working around the missing credentials. Happy to rebase or re-run whenever it suits. |
|
[blocking] The JavaScript package manifest still only includes |
|
Automated review note: I found line-specific issues in this PR, but this environment only exposes PR-level comment creation and not inline diff-comment posting. Per review policy, I’m not bundling line-specific findings into a PR-level comment. |
|
[blocking] |
dist staged only libconnector_service_ffi, so the gRPC client resolved a path no published tarball ever contained. Stage both natives per target via JS_NATIVE_LIBS. The release workflow built only libconnector_service_ffi, so staging the gRPC native without touching it would have tripped the existing fail-closed check and aborted every release. Build hyperswitch-grpc-ffi and upload libhyperswitch_grpc_ffi.* alongside it. package.json needs no change: npm-packlist includes a matched directory recursively, so the existing generated/* entry already ships the per-arch subdirectories. Verified against a real dist tarball.
|
Pushed
One wrinkle the suggestion doesn't cover:
And from a real Worth noting the per-arch layout isn't new in this PR — #1996 shipped it for the UniFFI native, so a glob that couldn't reach those subdirectories would already be visible in released packages today. Happy to switch to |
Follow-up to #1940, which reported two arch-blind native loaders in the JavaScript SDK. #1996 fixed the UniFFI one — it resolves
generated/<platform>-<arch>/now, and the release workflow buildsaarch64-unknown-linux-gnu. The gRPC loader was left as it was:No
process.arch, and a flat path with nowhere to put a second architecture. This brings it in line.Changes
sdk/javascript/src/payments/native_lib.ts(new): oneresolveNativeLib()shared by both loaders, so the platform/arch mapping can't drift between them._generated_grpc_client.tsresolves per-arch. The edit is intemplates/javascript/grpc_client.ts.j2, regenerated withgenerate.py --lang grpc; the Python, Kotlin and Rust outputs regenerate byte-identical.make generate-grpc-bindingsstages the native undergenerated/$(NODE_TARGET)/, the waygenerate-bindingsalready does, so staging and lookup agree.fileon the.soto find the mismatch; now it is:On scope
The gRPC part is consistency rather than a crash anyone hits today: staging and loading were both flat, so they agreed for any single-arch build. It starts to matter once a package carries more than one architecture, which is what
make distnow produces for the UniFFI native. The diagnostic is the part users feel immediately.One thing I ran into and left alone:
make -C sdk/javascript diststages onlylibconnector_service_ffi, andrelease-sdks.ymlnever callsgenerate-grpc-bindings, so no gRPC native ships in the npm tarball on any platform. Shipping a second native reads as a packaging decision rather than a loader fix, so I have not made it here. Happy to follow up if it is wanted.Testing
make -C sdk test-grpcin ci.yml covers this path end to end — it stages the native, packs a tarball, installs it, and loads through the resolver.Locally:
tsc --noEmitmatchesmain: the same two pre-existinggenerated/protoerrors, nothing new.process.platformandprocess.archoverridden. Each of linux-arm64, linux-x64 and darwin-arm64 resolves to its own directory and extension; aarch64 never falls through to the x86-64 binary; a stray flatgenerated/*.sois ignored; both failure paths produce the message above.make -C sdk/javascript generate-grpc-bindingsstages intogenerated/linux-x64/.I do not have aarch64 hardware, so this has not run against a real Graviton box — @alondayan2 offered to test an arm64 build in #1940.
Refs #1940